client/transport/inprocess.go (4)

57-66: Ensure sessionID is generated before creating/registering the session

If NewInProcessTransport() (without options) is used, sessionID remains empty. Creating a session with an empty ID can break RegisterSession/UnregisterSession lookups and GetSessionId. Generate an ID when missing and persist it.

Apply this diff:

 func (c *InProcessTransport) Start(ctx context.Context) error {
-	// Create and register session if we have handlers
-	if c.samplingHandler != nil || c.elicitationHandler != nil {
-		c.session = server.NewInProcessSessionWithHandlers(c.sessionID, c.samplingHandler, c.elicitationHandler)
+	// Create and register session if we have handlers
+	if c.samplingHandler != nil || c.elicitationHandler != nil {
+		// Ensure we have a session ID
+		if c.sessionID == "" {
+			c.sessionID = c.server.GenerateInProcessSessionID()
+		}
+		c.session = server.NewInProcessSessionWithHandlers(c.sessionID, c.samplingHandler, c.elicitationHandler)
 		if err := c.server.RegisterSession(ctx, c.session); err != nil {
 			return fmt.Errorf("failed to register session: %w", err)
 		}
 	}
 	return nil
 }

---

94-103: Preserve session context for notifications

Notifications should also carry the client session in the context to keep behavior consistent with requests (logging level, client info, etc.).

Apply this diff:

 func (c *InProcessTransport) SendNotification(ctx context.Context, notification mcp.JSONRPCNotification) error {
 	notificationBytes, err := json.Marshal(notification)
 	if err != nil {
 		return fmt.Errorf("failed to marshal notification: %w", err)
 	}
 	notificationBytes = append(notificationBytes, '\n')
+	// Add session to context if available
+	if c.session != nil {
+		ctx = c.server.WithContext(ctx, c.session)
+	}
 	c.server.HandleMessage(ctx, notificationBytes)

 	return nil
 }

---

111-116: Use actual session ID when unregistering

If sessionID wasn’t set on construction, UnregisterSession with an empty ID will fail. Fall back to the session’s ID.

Apply this diff:

 func (c *InProcessTransport) Close() error {
 	if c.session != nil {
-		c.server.UnregisterSession(context.Background(), c.sessionID)
+		sid := c.sessionID
+		if sid == "" {
+			sid = c.session.SessionID()
+		}
+		c.server.UnregisterSession(context.Background(), sid)
 	}
 	return nil
 }

---

118-120: Fix: Return actual session ID from GetSessionId()

Currently always returns an empty string.

Apply this diff:

 func (c *InProcessTransport) GetSessionId() string {
-	return ""
+	if c.sessionID != "" {
+		return c.sessionID
+	}
+	if c.session != nil {
+		return c.session.SessionID()
+	}
+	return ""
 }

server/stdio.go (2)

223-225: Same stdout write race in RequestSampling

Same issue as RequestElicitation: direct writes to the shared stdout without coordinating with the server’s write lock. Use the same writeWithGlobalLock helper to serialize all writes.

Apply this diff:

- if _, err := writer.Write(requestBytes); err != nil {
-   return nil, fmt.Errorf("failed to write sampling request: %w", err)
- }
+ if _, err := writeWithGlobalLock(writer, requestBytes); err != nil {
+   return nil, fmt.Errorf("failed to write sampling request: %w", err)
+ }

---

695-701: Unify server write path with the same global write lock to avoid interleaving with session-originated requests

writeResponse currently uses a different mutex (s.writeMu) than session-originated requests. To guarantee atomic frames across all writers, route all writes through the same global lock.

Apply this diff:

- // Protect concurrent writes
- s.writeMu.Lock()
- defer s.writeMu.Unlock()
-
- // Write response followed by newline
- if _, err := fmt.Fprintf(writer, "%s\n", responseBytes); err != nil {
-   return err
- }
+ // Write response followed by newline under shared lock
+ if _, err := writeWithGlobalLock(writer, append(responseBytes, '\n')); err != nil {
+   return err
+ }

Optionally, remove s.writeMu if no other code depends on it after this change.

mcp/types.go (3)

59-62: Introduce elicitation method: looks correct and consistent with naming.

Constant name and value are consistent with the rest of the method enum. Non-blocking nit: consider aligning the comment URL to the spec path once a canonical spec page exists.

---

834-841: Prefer a more precise type for RequestedSchema to improve safety and interop.

Using any works, but you’ll lose type guarantees and may incur unnecessary reflection. Two better options:

• Use json.RawMessage to preserve arbitrary JSON without reflection.
• If the repo already standardizes on a JSON Schema type (e.g., the Tool InputSchema handling per our past learnings), reuse that type for consistency.

Suggested minimal change:

-    RequestedSchema any `json:"requestedSchema"`
+    RequestedSchema json.RawMessage `json:"requestedSchema"`

---

842-860: Response envelope is well modeled; consider adding a helper validator.

The tri-state response with optional Value is clear. Consider adding a small helper (outside this file) to validate that Value is present iff Type is “accept” and to enforce schema-validation errors vs user-decline/cancel semantics. This will make downstream handler code simpler and safer.

server/session.go (1)

55-61: SessionWithElicitation interface is clear; document cancel vs error semantics.

Consider clarifying in a comment when to return an ElicitationResult with Type=cancel vs returning an error (e.g., transport interruption vs user-initiated cancel). This avoids ambiguity for implementers across transports.

client/elicitation.go (1)

9-19: ElicitationHandler contract looks good; add guidance on cancel/error handling.

Interface is minimal and clear. Non-blocking: add a sentence that user-initiated cancellations should produce a result with Type=cancel, while transport or context cancellations should return an error. This aligns client behavior with server expectations.

server/elicitation.go (1)

10-25: Server-side elicitation request routing is sound

Correctly retrieves the client session from context and delegates only if SessionWithElicitation is implemented. Error messages match tests.

Consider defining exported sentinel errors for "no active session" and "session does not support elicitation" to enable callers to handle them programmatically without string matching in the future. Keep current messages for test compatibility.

client/client.go (1)

527-573: Transport-level elicitation handling is correct and mirrors sampling

• Validates handler presence
• Parses params safely via JSON round-trip
• Constructs mcp.ElicitationRequest and returns properly marshaled result

Sampling and elicitation transport handlers share identical request-param parsing and response-marshaling patterns. Consider extracting a small generic helper to reduce duplication and error-prone maintenance (example skeleton):

// outside the changed ranges – illustrative
func (c *Client) handleBidi[TParams any, TResult any](
    ctx context.Context,
    req transport.JSONRPCRequest,
    build func(TParams) (any, error), // constructs the MCP request to call the handler with
    call func(context.Context, any) (TResult, error), // invokes the specific handler
) (*transport.JSONRPCResponse, error) {
    var params TParams
    if req.Params != nil {
        b, err := json.Marshal(req.Params)
        if err != nil {
            return nil, fmt.Errorf("failed to marshal params: %w", err)
        }
        if err := json.Unmarshal(b, &params); err != nil {
            return nil, fmt.Errorf("failed to unmarshal params: %w", err)
        }
    }
    mcpReq, err := build(params)
    if err != nil {
        return nil, err
    }
    result, err := call(ctx, mcpReq)
    if err != nil {
        return nil, err
    }
    rb, err := json.Marshal(result)
    if err != nil {
        return nil, fmt.Errorf("failed to marshal result: %w", err)
    }
    return &transport.JSONRPCResponse{
        JSONRPC: mcp.JSONRPC_VERSION,
        ID:      req.ID,
        Result:  json.RawMessage(rb),
    }, nil
}

This keeps both flows uniform and easier to extend (e.g., future bidirectional methods).

server/elicitation_test.go (1)

114-172: Happy-path test validates Accept flow and structured Value

Asserts type and content of response payload; representative example of structured elicitation.

You could add an additional sub-assert that Value conforms to the declared schema in the test (e.g., check both fields exist and are strings) to better document intended behavior.

server/stdio.go (3)

629-683: Tighten response discrimination to avoid false positives

If a message has an id but neither result nor error, this method will parse, attempt to unmarshal nil, and treat it as an elicitation response error. Align this with handleSamplingResponse by ignoring such frames.

Apply this diff:

- id, err := response.ID.Int64()
- if err != nil {
-   return false
- }
+ id, err := response.ID.Int64()
+ if err != nil || (response.Result == nil && response.Error == nil) {
+   return false
+ }

---

423-446: Potential goroutine leak on context cancel in readNextLine

When ctx is cancelled, this returns "", nil but leaves the spawned goroutine blocked on ReadString. This can leak goroutines until stdin closes. Consider refactoring to avoid a goroutine per read or to unblock the reader on cancel (e.g., by using an interruptible reader, piping through an io.Pipe on shutdown, or a single long-lived goroutine that you signal to stop).

I can draft a refactor using a single read goroutine feeding a channel that the loop selects on, with proper shutdown semantics.

---

327-333: Nit: comment contradicts behavior

Comment says “Default to discarding logs” but logs are directed to os.Stderr.

Apply this diff to clarify:

- ), // Default to discarding logs
+ ),

Or update the comment to match the behavior.

client/inprocess_elicitation_test.go (5)

23-33: Prefer map[string]any over map[string]interface{} for consistency

Throughout the repository, any is used. Keep tests consistent to avoid noise in diffs and improve readability.

Apply this diff:

-       Value: map[string]interface{}{
+       Value: map[string]any{
          "response": "User provided data",
          "accepted": true,
        },

---

39-53: Minor: use map[string]any consistently in schema definitions

The file mixes map[string]any and map[string]interface{}. Align on any.

Apply this diff:

-   Properties: map[string]any{
-     "action": map[string]any{
+   Properties: map[string]any{
+     "action": map[string]any{
        "type":        "string",
        "description": "Action to perform",
      },
    },

And within the elicitation request:

- RequestedSchema: map[string]interface{}{
+ RequestedSchema: map[string]any{
     "type": "object",
-    "properties": map[string]interface{}{
-      "confirm": map[string]interface{}{
+    "properties": map[string]any{
+      "confirm": map[string]any{
         "type":        "boolean",
         "description": "Confirm the action",
       },
-      "details": map[string]interface{}{
+      "details": map[string]any{
         "type":        "string",
         "description": "Additional details",
       },
     },

---

193-197: Prefer public option to set handler rather than mutating unexported field

Setting client.elicitationHandler directly couples the test to internal details. Use the functional option to exercise public API.

Apply this diff:

- client := NewClient(inProcessTransport)
- client.elicitationHandler = handler
+ client := NewClient(inProcessTransport, WithElicitationHandler(handler))

---

80-93: Coverage: add a failing elicitation path test

The tool path already returns a CallToolResult with IsError true when RequestElicitation fails. Add a test that injects a failing handler or forces RequestElicitation to error and assert the error content and IsError=true.

I can add a subtest that makes the handler return an error and asserts the tool returns a text error and IsError=true.

---

171-173: String stability

Tests compare exact strings. Consider asserting on the response type (Accept/Decline/Cancel) and presence of substrings to reduce brittleness to minor text changes.

I can adjust the assertion to check Contains("User accepted") instead of full equality if desired.

client/elicitation_test.go (1)

84-123: Strengthen assertions by checking ID echo and response payload

After unmarshalling, also assert that the response type matches the test case and that the Request/Response IDs are echoed back.

For example:

if result != nil {
  if result.ID != request.ID {
    t.Errorf("expected response ID %v, got %v", request.ID, result.ID)
  }
  var elicitationResult mcp.ElicitationResult
  if err := json.Unmarshal(result.Result, &elicitationResult); err != nil {
    t.Errorf("failed to unmarshal result: %v", err)
  } else if tt.expectedError == "" && elicitationResult.Response.Type == "" {
    t.Error("empty response type")
  }
}

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these settings in your CodeRabbit configuration.

Learnt from: xinwo
PR: mark3labs/mcp-go#35
File: mcp/tools.go:107-137
Timestamp: 2025-03-04T06:59:43.882Z
Learning: Tool responses from the MCP server shouldn't contain RawInputSchema, which is why the UnmarshalJSON method for the Tool struct is implemented to handle only the structured InputSchema format.

Learnt from: ezynda3
PR: mark3labs/mcp-go#461
File: server/sampling.go:22-26
Timestamp: 2025-06-30T07:13:17.052Z
Learning: In the mark3labs/mcp-go project, the MCPServer.capabilities field is a struct value (serverCapabilities), not a pointer, so it cannot be nil and doesn't require nil checking. Only pointer fields within the capabilities struct should be checked for nil.

server/session.go (1)

55-61: Interface addition is appropriate; clarify cancellation semantics and fix doc grammar.

Good place for the elicitation extension. Tighten the comment to specify context behavior and correct wording.

-// SessionWithElicitation is an extension of ClientSession that can send elicitation requests
+// SessionWithElicitation is an extension of ClientSession that can send elicitation requests.
 type SessionWithElicitation interface {
 	ClientSession
-	// RequestElicitation sends an elicitation request to the client and waits for response
+	// RequestElicitation sends an elicitation request to the client and waits for a response.
+	// Implementations MUST honor ctx cancellation/deadline and return an error consistent with ctx.Err().
 	RequestElicitation(ctx context.Context, request mcp.ElicitationRequest) (*mcp.ElicitationResult, error)
 }

Optionally, add a typed accessor to reduce repetitive type assertions at call sites:

// SessionWithElicitationFromContext retrieves a SessionWithElicitation from context, if available.
func SessionWithElicitationFromContext(ctx context.Context) (SessionWithElicitation, bool) {
	cs := ClientSessionFromContext(ctx)
	swe, ok := cs.(SessionWithElicitation)
	return swe, ok
}

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these settings in your CodeRabbit configuration.

examples/elicitation/main.go (3)

56-60: Good: guarded assertion prevents panics on malformed elicitation response.

This addresses prior feedback about unsafe map assertions and avoids runtime panics.

---

121-131: Good: switched to safe parameter access.

Using request.GetString("data", "") avoids unsafe type assertions and is robust when params are missing.

---

165-168: Don’t proceed on malformed accept payload; fail fast.

Falling back to an empty map will treat malformed responses as declines/missing values. Return an error to make issues explicit.

-				responseData, ok := result.Response.Value.(map[string]interface{})
-				if !ok {
-					responseData = make(map[string]interface{})
-				}
+				responseData, ok := result.Response.Value.(map[string]interface{})
+				if !ok {
+					return nil, fmt.Errorf("unexpected response format")
+				}

examples/elicitation/main.go (4)

22-42: Tighten schema: disallow extraneous fields.

Add additionalProperties: false so clients don’t send unexpected fields.

 				RequestedSchema: map[string]interface{}{
 					"type": "object",
 					"properties": map[string]interface{}{
 						"projectName": map[string]interface{}{
 							"type":        "string",
 							"description": "Name of the project",
 							"minLength":   1,
 						},
 						"framework": map[string]interface{}{
 							"type":        "string",
 							"description": "Frontend framework to use",
 							"enum":        []string{"react", "vue", "angular", "none"},
 						},
 						"includeTests": map[string]interface{}{
 							"type":        "boolean",
 							"description": "Include test setup",
 							"default":     true,
 						},
 					},
+					"additionalProperties": false,
 					"required": []string{"projectName"},
 				},

---

157-163: Differentiate Decline vs Cancel; return an error on cancellation.

Lumping all non-accept responses into “cancelled” loses semantics and forces clients to parse text. Treat Decline as a non-error result and Cancel as an error.

-				if result.Response.Type != mcp.ElicitationResponseTypeAccept {
-					return &mcp.CallToolResult{
-						Content: []mcp.Content{
-							mcp.NewTextContent("Processing cancelled by user"),
-						},
-					}, nil
-				}
+				switch result.Response.Type {
+				case mcp.ElicitationResponseTypeAccept:
+					// continue to validate fields below
+				case mcp.ElicitationResponseTypeDecline:
+					return &mcp.CallToolResult{
+						Content: []mcp.Content{
+							mcp.NewTextContent("Processing declined by user"),
+						},
+					}, nil
+				case mcp.ElicitationResponseTypeCancel:
+					return nil, fmt.Errorf("processing cancelled by user")
+				default:
+					return nil, fmt.Errorf("unexpected response type: %s", result.Response.Type)
+				}

---

136-149: Tighten schema for confirmation request.

Disallow extra fields to keep the contract tight.

 						RequestedSchema: map[string]interface{}{
 							"type": "object",
 							"properties": map[string]interface{}{
 								"proceed": map[string]interface{}{
 									"type":        "boolean",
 									"description": "Whether to proceed with processing",
 								},
 								"reason": map[string]interface{}{
 									"type":        "string",
 									"description": "Optional reason for your decision",
 								},
 							},
+							"additionalProperties": false,
 							"required": []string{"proceed"},
 						},

---

202-204: Handle SIGTERM for graceful shutdown in more environments.

Add syscall.SIGTERM to support container orchestrators.

-	signal.Notify(sigChan, os.Interrupt)
+	signal.Notify(sigChan, os.Interrupt, syscall.SIGTERM)

Additional change outside the selected range:

// in the imports block
"syscall"

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these settings in your CodeRabbit configuration.

Learnt from: lariel-fernandes
PR: mark3labs/mcp-go#428
File: www/docs/pages/servers/prompts.mdx:218-234
Timestamp: 2025-06-20T20:39:51.870Z
Learning: In the mcp-go library, the GetPromptParams.Arguments field is of type map[string]string, not map[string]interface{}, so direct string access without type assertions is safe and correct.

client/transport/streamable_http.go (4)

598-601: Per-attempt connect context no longer times out — consider a bounded attempt duration

Switching to context.WithCancel removes the previous per-attempt timeout. If the server stalls accepting the GET stream, this attempt can hang indefinitely until the parent ctx is canceled. For robustness (especially on flaky networks), keep a bounded attempt duration (e.g., 10s) and retry, while still respecting the parent ctx.

Apply this diff to reintroduce a bounded connect timeout:

-        connectCtx, cancel := context.WithCancel(ctx)
+        connectCtx, cancel := context.WithTimeout(ctx, 10*time.Second)
         err := c.createGETConnectionToServer(connectCtx)
         cancel()

If you’d prefer configurability, introduce an option to override the attempt timeout.

---

698-701: Hardcoded 30s timeout for server-initiated requests is likely too short for elicitation

Elicitations often require user input and can exceed 30 seconds. A fixed 30s deadline risks spurious cancellations.

Make the timeout configurable (transport option) or inherit a deadline from ctx if set, falling back to a sane default. Example:

-        requestCtx, cancel := context.WithTimeout(ctx, 30*time.Second)
+        deadlineCtx := ctx
+        if _, hasDeadline := ctx.Deadline(); !hasDeadline {
+            // Consider pulling from a configurable option instead of hardcoding
+            deadlineCtx, _ = context.WithTimeout(ctx, 2*time.Minute)
+        }
+        requestCtx, cancel := context.WithCancel(deadlineCtx)

---

710-716: Differentiate JSON-RPC error codes for cancel vs timeout

Both context.Canceled and context.DeadlineExceeded map to -32800. JSON-RPC 2.0 extensions commonly differentiate:

• -32800: Request cancelled
• -32802: Request timeout

Apply this diff:

-            if errors.Is(err, context.Canceled) {
-                errorCode = -32800 // Request cancelled
-                errorMessage = "request was cancelled"
-            } else if errors.Is(err, context.DeadlineExceeded) {
-                errorCode = -32800 // Request timeout
-                errorMessage = "request timed out"
+            if errors.Is(err, context.Canceled) {
+                errorCode = -32800 // Request cancelled
+                errorMessage = "request was cancelled"
+            } else if errors.Is(err, context.DeadlineExceeded) {
+                errorCode = -32802 // Request timeout
+                errorMessage = "request timed out"

---

473-529: SSE parser only stores the last data: line — consider supporting multi-line SSE data frames

SSE events may include multiple data: lines that must be concatenated with newline. Current logic overwrites data with the last line. This can truncate larger JSON payloads.

Here is a lightweight improvement:

-    var event, data string
+    var event string
+    var dataLines []string
...
-            if line == "" {
+            if line == "" {
                 // Empty line means end of event
-                if data != "" {
+                if len(dataLines) > 0 {
                     // If no event type is specified, use empty string (default event type)
                     if event == "" {
                         event = "message"
                     }
-                    handler(event, data)
-                    event = ""
-                    data = ""
+                    payload := strings.Join(dataLines, "\n")
+                    handler(event, payload)
+                    event = ""
+                    dataLines = nil
                 }
                 continue
             }
...
-            } else if strings.HasPrefix(line, "data:") {
-                data = strings.TrimSpace(strings.TrimPrefix(line, "data:"))
+            } else if strings.HasPrefix(line, "data:") {
+                dataLines = append(dataLines, strings.TrimSpace(strings.TrimPrefix(line, "data:")))
             }

server/elicitation.go (1)

10-25: Server-side RequestElicitation entry point — LGTM; consider typed errors

Flow and checks are correct. For better programmatic handling, consider defining sentinels:

• ErrNoActiveSession
• ErrSessionDoesNotSupportElicitation

and return them (wrapping with fmt.Errorf as needed) instead of stringly errors.

I can add these with tests if you’d like.

server/streamable_http.go (3)

239-247: Unified response handling path — LGTM; update error text to reflect generalized responses

The code/log messages still mention “sampling response”. Consider neutral wording (“request response”) now that elicitation uses the same path.

Apply these string tweaks locally in this handler and handleResponse.

---

542-609: Generic response handling is solid; adjust sampling-specific wording and tighten error messages

• Messages like “Missing session ID for sampling response” are now misleading for elicitation.
• The “sampling response has neither result nor error” case should be generalized.

Apply this diff:

-    http.Error(w, "Missing session ID for sampling response", http.StatusBadRequest)
+    http.Error(w, "Missing session ID for client response", http.StatusBadRequest)
...
-            response.err = fmt.Errorf("sampling error %d: %s", jsonrpcError.Code, jsonrpcError.Message)
+            response.err = fmt.Errorf("client response error %d: %s", jsonrpcError.Code, jsonrpcError.Message)
...
-    response.err = fmt.Errorf("sampling response has neither result nor error")
+    response.err = fmt.Errorf("client response has neither result nor error")

---

872-915: RequestElicitation mirrors sampling — LGTM; fix copy/paste comment and message text

• Comment says “Create the sampling request item”.
• Error messages should not say “sampling” in the elicitation path.

Apply this diff:

-    // Create the sampling request item
+    // Create the elicitation request item
...
-        return nil, fmt.Errorf("elicitation request queue is full - server overloaded")
+        return nil, fmt.Errorf("elicitation request queue is full - server overloaded")

(Also consider updating any remaining “sampling” wording in this block to “elicitation” where applicable.)

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

Learnt from: leavez
PR: mark3labs/mcp-go#114
File: client/transport/sse.go:137-179
Timestamp: 2025-04-06T10:07:06.685Z
Learning: The SSE client implementation in the MCP-Go project uses a 30-second timeout for reading SSE events to match the behavior of the original implementation before the transport layer refactoring.

Learnt from: ezynda3
PR: mark3labs/mcp-go#461
File: server/sampling.go:22-26
Timestamp: 2025-06-30T07:13:17.052Z
Learning: In the mark3labs/mcp-go project, the MCPServer.capabilities field is a struct value (serverCapabilities), not a pointer, so it cannot be nil and doesn't require nil checking. Only pointer fields within the capabilities struct should be checked for nil.

client/client.go (1)

577-584: Ping handler now returns a proper JSON-RPC success with echoed ID

This addresses the earlier review about wrapping the ID and omitting result; the response now echoes request.ID and includes a non-empty “result”. Nice fix.

Consider handling the unlikely marshal error (even though marshaling an empty struct won’t fail) to be consistent with surrounding code:

-func (c *Client) handlePingRequestTransport(ctx context.Context, request transport.JSONRPCRequest) (*transport.JSONRPCResponse, error) {
-    b, _ := json.Marshal(&mcp.EmptyResult{})
+func (c *Client) handlePingRequestTransport(ctx context.Context, request transport.JSONRPCRequest) (*transport.JSONRPCResponse, error) {
+    b, err := json.Marshal(&mcp.EmptyResult{})
+    if err != nil {
+        return nil, fmt.Errorf("failed to marshal ping result: %w", err)
+    }
     return &transport.JSONRPCResponse{
         JSONRPC: mcp.JSONRPC_VERSION,
         ID:      request.ID,
         Result:  b,
     }, nil
 }

server/streamable_http.go (4)

240-247: Generic response handling: fix sampling-specific log/error messages

You converted response handling to a generic path; the log and HTTP error messages still say “sampling response”. Update to “client response”.

-        if err := s.handleResponse(w, r, jsonMessage); err != nil {
-            s.logger.Errorf("Failed to handle sampling response: %v", err)
-            http.Error(w, "Failed to handle sampling response", http.StatusInternalServerError)
-        }
+        if err := s.handleResponse(w, r, jsonMessage); err != nil {
+            s.logger.Errorf("Failed to handle client response: %v", err)
+            http.Error(w, "Failed to handle client response", http.StatusInternalServerError)
+        }

---

426-433: Use protocol constant for JSONRPC version

Hardcoding "2.0" is brittle. Prefer mcp.JSONRPC_VERSION for both sampling and elicitation SSE requests.

-                jsonrpcRequest := mcp.JSONRPCRequest{
-                    JSONRPC: "2.0",
+                jsonrpcRequest := mcp.JSONRPCRequest{
+                    JSONRPC: mcp.JSONRPC_VERSION,
                     ID:      mcp.NewRequestId(samplingReq.requestID),
                     Request: mcp.Request{
                         Method: string(mcp.MethodSamplingCreateMessage),
                     },
                     Params: samplingReq.request.CreateMessageParams,
                 }

-                jsonrpcRequest := mcp.JSONRPCRequest{
-                    JSONRPC: "2.0",
+                jsonrpcRequest := mcp.JSONRPCRequest{
+                    JSONRPC: mcp.JSONRPC_VERSION,
                     ID:      mcp.NewRequestId(elicitationReq.requestID),
                     Request: mcp.Request{
                         Method: string(mcp.MethodElicitationCreate),
                     },
                     Params: elicitationReq.request.Params,
                 }

Also applies to: 441-448

---

542-608: Make handleResponse messages generic (not sampling-specific)

These user-facing and log messages still mention “sampling”. Make them generic to cover elicitation too.

-    http.Error(w, "Missing session ID for sampling response", http.StatusBadRequest)
+    http.Error(w, "Missing session ID for client response", http.StatusBadRequest)
@@
-    http.Error(w, "Invalid request ID in sampling response", http.StatusBadRequest)
+    http.Error(w, "Invalid request ID in client response", http.StatusBadRequest)
@@
-            response.err = fmt.Errorf("sampling error %d: %s", jsonrpcError.Code, jsonrpcError.Message)
+            response.err = fmt.Errorf("client error %d: %s", jsonrpcError.Code, jsonrpcError.Message)
@@
-        response.err = fmt.Errorf("sampling response has neither result nor error")
+        response.err = fmt.Errorf("response has neither result nor error")

---

124-124: Outdated comment: responses are no longer sampling-only

Update the comment to reflect unified delivery for sampling and elicitation (and ping drops).

-activeSessions    sync.Map // sessionId --> *streamableHttpSession (for sampling responses)
+activeSessions    sync.Map // sessionId --> *streamableHttpSession (for bidirectional responses)

server/streamable_http_sampling_test.go (1)

178-216: Add coverage for heartbeat responses and ID uniqueness

Given the unified response path:

• Verify that a ping response does not 500 when there’s an active session but no pending request.
• Verify request IDs don’t collide between heartbeat and sampling/elicitation after unifying the generator.

I can draft a focused unit test that registers an active session, calls deliverResponse with a non-existent request ID, and asserts 200 OK behavior by having handleResponse return nil (i.e., no HTTP write). Alternatively, we can integration-test the ping flow by enabling WithHeartbeatInterval and asserting the POSTed ping response is accepted (200) while a sampling request concurrently succeeds.

Want me to add such tests in a follow-up commit?

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.